Swapped out to_string_lossy with display for user facing text by tiensi · Pull Request #5666 · block/goose

tiensi · 2025-11-11T02:13:26Z

Summary

Replaced user facing to_string_lossy with display calls as recommended per issue here

Type of Change

AI Assistance

This PR was created or reviewed with AI assistance

Testing

N/A

Related Issues

Relates to #5435

tiensi · 2025-11-11T02:14:50Z

crates/goose-cli/src/commands/project.rs

        for component in components.iter().skip(len - 2) {
            path_str.push('/');
-            path_str.push_str(component.as_os_str().to_string_lossy().as_ref());
+            path_str.push_str(component.as_os_str().display().as_ref());


displayed in commandline later in line 76

crates/goose-cli/src/commands/project.rs

crates/goose-cli/src/commands/session.rs

tiensi · 2025-11-11T02:18:57Z

crates/goose-cli/src/recipes/search_recipe.rs

                name,
                source: RecipeSource::Local,
-                path: path.to_string_lossy().to_string(),
+                path: path.display().to_string(),


displayed later in commands/recipe.rs

tiensi · 2025-11-11T02:20:14Z

crates/goose-cli/src/cli.rs

                                goose::recipe::template_recipe::parse_recipe_content(
                                    &rf.content,
-                                    Some(rf.parent_dir.to_string_lossy().to_string()),
+                                    Some(rf.parent_dir.display().to_string()),


Used for tracing (printing?) in line 1120

crates/goose-mcp/src/developer/text_editor.rs

tiensi · 2025-11-11T02:24:27Z

crates/goose/src/recipe/local_recipes.rs

    let search_dirs_str = search_dirs
        .iter()
-        .map(|p| p.to_string_lossy())
+        .map(|p| p.display().to_string())


part of error on line 67. Not 100% certain whether errors are consumed by user.

Copilot

Pull Request Overview

This PR attempts to replace to_string_lossy() with display() for user-facing path text across multiple files to improve handling of non-UTF-8 paths. However, the implementation introduces compilation errors by calling .display() on OsStr/OsString types which don't have this method.

Replaces to_string_lossy() with display() in 6 files
Affects error messages, directory listings, and path formatting
Changes span across CLI, MCP, and core recipe loading code

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
crates/goose/src/recipe/local_recipes.rs	Updates error message path formatting in recipe file loading
crates/goose-mcp/src/developer/text_editor.rs	Changes directory entry name handling
crates/goose-cli/src/recipes/search_recipe.rs	Updates path display in recipe listing
crates/goose-cli/src/commands/session.rs	Modifies path filtering in session list command
crates/goose-cli/src/commands/project.rs	Changes path component formatting in two locations
crates/goose-cli/src/cli.rs	Updates recipe parent directory path handling

crates/goose-mcp/src/developer/text_editor.rs

crates/goose-cli/src/commands/project.rs

Signed-off-by: Vincent Huang <vhuang@squareup.com>

Copilot

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

crates/goose-cli/src/commands/project.rs

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copilot · 2025-11-11T02:57:53Z

crates/goose/src/recipe/local_recipes.rs

    let search_dirs_str = search_dirs
        .iter()
-        .map(|p| p.to_string_lossy())
+        .map(|p| p.display().to_string())
        .collect::<Vec<_>>()
        .join(":");


The hardcoded ":" separator on line 68 is incorrect for Windows. The code uses platform-specific separators (';' for Windows, ':' for Unix) when parsing GOOSE_RECIPE_PATH on line 26. Use the same logic here: let path_separator = if cfg!(windows) { ';' } else { ':' }; and then .join(path_separator).

Don't want to change the scope of this pr (I'm assuming this change will require adding unit tests and normalizing the if cfg!(windows)... logic for reuse), will ignore for now.

copilot is wrong; this is used interally

* origin/main: (29 commits) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) feat: add Supabase MCP server to registry (#5629) Unlist VS Code extension tutorials from MCP and experimental sections (#5677) fix: make image processing work in github copilot provider (#5687) fix: do not take into account gitignore in developer mcp (#5688) docs: session storage migration (#5682) New maintainers (#5685) chore: Update governance (#5660) chore(release): release version 1.14.0 (minor) (#5676) fix : action icons overlap session title in chat history (#5684) Document recent goose PRs (#5683) docs: add GOOSE_PATH_ROOT environment variable documentation (#5678) feat: SessionManager integration for acp sessions (#5657) teach copilot our CI (#5672) bump openapi version directly (#5674) governance: update MAINTAINERS.md to reflect new maintainers (#5675) chore: upgrade rmcp to 0.8.5 (#5673) Update release instructions (#5662) Swapped out to_string_lossy with display for user facing text (#5666) ...

…5666) Signed-off-by: Vincent Huang <vhuang@squareup.com>

…5666) Signed-off-by: Vincent Huang <vhuang@squareup.com> Signed-off-by: Blair Allan <Blairallan@icloud.com>

tiensi commented Nov 11, 2025

View reviewed changes

crates/goose-cli/src/commands/project.rs Outdated Show resolved Hide resolved

tiensi commented Nov 11, 2025

View reviewed changes

crates/goose-cli/src/commands/session.rs Outdated Show resolved Hide resolved

tiensi commented Nov 11, 2025

View reviewed changes

crates/goose-mcp/src/developer/text_editor.rs Outdated Show resolved Hide resolved

tiensi commented Nov 11, 2025

View reviewed changes

tiensi marked this pull request as ready for review November 11, 2025 02:29

Copilot AI review requested due to automatic review settings November 11, 2025 02:29

Copilot started reviewing on behalf of tiensi November 11, 2025 02:30 View session

tiensi mentioned this pull request Nov 11, 2025

Prefer display() over to_string_lossy() when showing paths #5435

Closed

Copilot finished reviewing on behalf of tiensi November 11, 2025 02:30

Copilot AI reviewed Nov 11, 2025

View reviewed changes

crates/goose-mcp/src/developer/text_editor.rs Outdated Show resolved Hide resolved

crates/goose-cli/src/commands/project.rs Outdated Show resolved Hide resolved

crates/goose-cli/src/commands/project.rs Outdated Show resolved Hide resolved

Replaced user facing paths with display

07c2156

Signed-off-by: Vincent Huang <vhuang@squareup.com>

tiensi force-pushed the tiensi/issue-5435-cleanup-display-and-lossy branch from 0080343 to 07c2156 Compare November 11, 2025 02:32

Cargo format

8e50228

Signed-off-by: Vincent Huang <vhuang@squareup.com>

Copilot AI review requested due to automatic review settings November 11, 2025 02:33

Copilot started reviewing on behalf of tiensi November 11, 2025 02:34 View session

Copilot finished reviewing on behalf of tiensi November 11, 2025 02:35

Copilot AI reviewed Nov 11, 2025

View reviewed changes

crates/goose-cli/src/commands/project.rs Outdated Show resolved Hide resolved

tiensi added 2 commits November 10, 2025 18:36

Removed display changes for OsStr

f615d52

Additional cleanup

0dcf60b

Copilot AI review requested due to automatic review settings November 11, 2025 02:55

Copilot started reviewing on behalf of tiensi November 11, 2025 02:56 View session

Copilot finished reviewing on behalf of tiensi November 11, 2025 02:57

Copilot AI reviewed Nov 11, 2025

View reviewed changes

DOsinga approved these changes Nov 11, 2025

View reviewed changes

DOsinga merged commit 23f26fb into block:main Nov 11, 2025
20 checks passed

github-actions bot mentioned this pull request Nov 11, 2025

chore(release): release version 1.14.0 (minor) #5676

Merged

Surendhar-N-D pushed a commit to Surendhar-N-D/goose that referenced this pull request Nov 17, 2025

Swapped out to_string_lossy with display for user facing text (block#…

88012ba

…5666) Signed-off-by: Vincent Huang <vhuang@squareup.com>

arul-cc pushed a commit to arul-cc/goose that referenced this pull request Nov 17, 2025

Swapped out to_string_lossy with display for user facing text (block#…

456d108

…5666) Signed-off-by: Vincent Huang <vhuang@squareup.com>

BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025

Swapped out to_string_lossy with display for user facing text (block#…

5609dfb

…5666) Signed-off-by: Vincent Huang <vhuang@squareup.com> Signed-off-by: Blair Allan <Blairallan@icloud.com>

Conversation

tiensi commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of Change

AI Assistance

Testing

Related Issues

Uh oh!

tiensi Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tiensi Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

tiensi Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tiensi Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

tiensi Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

DOsinga Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tiensi commented Nov 11, 2025 •

edited

Loading